fix(udp): expose UDP ports properly#3485
Conversation
✅ Deploy Preview for testcontainers-go ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Summary by CodeRabbitBug Fixes
Tests
WalkthroughA fix is applied to the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@blueprismo i think we need to update the tests for the new condition. Can you take a look? 🙏 |
02d81d8 to
4d3ab52
Compare
What does this PR do?
Fixes UDP port exposure.
Problem:
When using ExposedPorts: []string{"8080/udp"} in testcontainers-go, the MappedPort() function always returned "0/udp" instead of the actual mapped host port (e.g., "55051/udp"). This made it impossible to connect to UDP services running in containers.
Root Cause:
The issue was in the mergePortBindings() function in lifecycle.go. When nat.ParsePortSpecs() processes exposed port specifications like 8080/udp, it creates PortBinding structs with empty HostPort fields ({HostIP: "", HostPort: ""})
Docker interprets these differently:
HostPort: "" (empty) → "Don't bind this port to any host port" → results in port 0
HostPort: "0" → "Bind this port to a random available host port" → proper allocation
Solution:
Modified mergePortBindings() to convert empty HostPort values to "0" for automatic port allocation:
Why is it important?
To support UDP connections correctly as now they aren't useful (at least with random port allocatio
Related issues
Follows: #3484